Skip to content

Conversation

@SabitaShrestha325
Copy link

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with COHORT_NAME | FIRST_NAME LAST_NAME | REPO_NAME | WEEK
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

@SabitaShrestha325 SabitaShrestha325 changed the title Coursework planner sprint 1 ITP_JAN-2015|sabita-Shrestha|Module-Structuring-and-Testing-Data|week5|sprint1 Feb 6, 2025
@SabitaShrestha325 SabitaShrestha325 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 7, 2025
@cjyuan cjyuan added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label Feb 12, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you probably understand the code.

Describing code can be very challenging initially (even in one's first language). May I suggest using ChatGPT to find out how else code can be described? From time to time you may also learn some useful programming terms from ChatGPT.


// Line 1 is a variable declaration, creating the count variable with an initial value of 0
// Describe what line 3 is doing, in particular focus on what = is doing
It will evaluate the right hand side argument and add the varaible which is 0+1 = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably understand what the code does.
However, using the correct terminology is crucial when communicating with other programmers.
May I suggest (with the help of ChatGPT)

  • Find out how else the code can be described
  • Look up the terms "expression", "operand", "argument" in programming
    ?


// https://www.google.com/search?q=get+first+character+of+string+mdn

//text.charAt(0) means it will print first letter, if we put 1 than it will print second letter.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A function "returns" (not "prints") a value.


//When you log several time it will so different value.
// Math.random gives u the value with floating number like 0.38
// But Math.floor go around give largest integer ex. if value is 6.95 it will only print 6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you shed more details by answer the following questions?

  • What is the range of values that could be returned by Math.random()?
  • What is the significance of (maximum - minimum + 1)? What does this expression represent?
  • What is the significance of ... + minimum?
  • What is the range of values that could be assigned to num?

To test your understanding, how would you write an expression (without using any variable in the expression) that can yield a random integer between -5 and 5 (including both -5 and 5)?

Comment on lines 1 to 4
const twelveHourClockTime = "20:53";
const twentyFourhourClockTime = "08:53";
console.log(twelveHourClockTime);
console.log(twentyFourhourClockTime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you also noticed the variable names do not quite match the values assigned to the variable?

Comment on lines 1 to 3
const cardNumber = "4533787178994213";
const last4Digits = cardNumber.slice(-4);

console.log(last4Digits);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you cannot modify the statement const cardNumber = 4533787178994213;
(that is, keep the variable's value unchanged),
how would you modify the code (through type conversion) so that it can still extract the last 4 digits from its value.

//ans: This is called remainder operator which divide the length of movie 8784 by 60 and place the remaining second which is 24.

// d) Interpret line 4, what does the expression assigned to totalMinutes mean?
// ans: It means movie length subtract the remaining seconds and use remainder operator like (8784-24=8760/60) which 146.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You gave a literal translation of the code, but it does not quite explain what the expression (movieLength - remainingSeconds) / 60 does.

Can you describe in terms of whole minute, or use ChatGPT to find out how else the code can be described?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, Cjyuan, thank you for your review, I have changed the ans as it was mention. Can you please review.

// ans: It means movie length subtract the remaining seconds and use remainder operator like (8784-24=8760/60) which 146.

// e) What do you think the variable result represents? Can you think of a better name for this variable?
// ans I think its define time of the movie length in hour, minute, and second. Better name for this variable will be "timeString".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"time" can ambiguous because it may mean refer to clock time (e.g., 12:30pm), time in seconds, etc.
Can you think of another name for the variable?

Comment on lines 31 to 32
//In this line, trailing comma is removed from the penceString and substring remove the first line(0)
// and one character before the end which is (penceString.length-1) it will left "399"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • There is no "trailing comma" in penceString.
  • .substring() does not actually remove anything from a string; it only returns "part" of a string.
  • What do you mean by "first line"?

Can you rephrase this description?

//This line remove the part of the string which is pound and substring from the start(0) to characters before the end paddedPenceNumberString.

//const pence = paddedPenceNumberString .substring(paddedPenceNumberString.length - 2) .padEnd(2, "0");
//This line of code extracts 2 characters of the string "paddedPenceNumberString" also pads the results to make sure it has 2 characters by adding 0 to format properly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which 2 characters are extracted from paddedPenceNumberString?

//This line make sure the string "penceStringWithoutTrailingP" has 3 character and also adding "0" if necessary and it also penceStringWithoutTrailingP is "99" also padded to "099".

//const pounds = paddedPenceNumberString.substring(0, paddedPenceNumberString.length - 2);
//This line remove the part of the string which is pound and substring from the start(0) to characters before the end paddedPenceNumberString.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by "... to characters before the end paddedPenceNumberString"?

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Feb 13, 2025
@SabitaShrestha325 SabitaShrestha325 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 15, 2025
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement.

There are still a few descriptions you can improve.

// ans: It was SyntaxError: missing ) which is replaceAll(",", "")comma separate argument.

// c) Identify all the lines that are variable reassignment statements
// c) Identify all the lines that are variable assignment statements
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of changing the description of the question, your goal is supposed to identify the statements where a variable is re-assigned a value.

let a = 1;   // assign a value to variable a
a = 2;       // re-assign a value to variable a (this is a reassignment statement)
a = a + 1;   // re-assign a value to variable a (this is also a reassignment statement)

// e) Describe what the expression Number(carPrice.replaceAll(",","")) is doing - what is the purpose of this expression?
// ans: It is value in string which is car price(10,000) and priceAfterOneYear(8,543), also replace and have new string value.
// ans: The expression Number(carPrice.replaceAll(",", "")) is used to convert a formatted string representation of a number (with commas) into an actual numeric value.
//Basically Number(carPrice.replaceAll(",","")) remove comma, remove strings and allows number to use mathematical.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your description at line 34 is correct.
However, I don't know what you meant by "remove strings" in the statement at line 35.

The two function calls involved in the statement carPrice = Number(carPrice.replaceAll(",", "")) can be broken down into the following code (to demonstrate the order in which the function are called).

const strWithoutComma = carPrice.replaceAll(",", "");
carPrice = Number(strWithoutComma);

Comment on lines 31 to 34
//In this line, penceString is string with trailing p so to extract p substring is used and penceString.length - 1,
// will be p which will be extract.( penceString.length is 4(399p) penceString.length - 1 will extract end of the length which is p)
//so out put will be 399.
//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These statements are not very clear.
Extract "p" would suggest taking out "p" and return "p" from the given string.
Can you rephrase it? ChatGPT can probably help.

@SabitaShrestha325
Copy link
Author

Great improvement.

There are still a few descriptions you can improve.

Thank you for your review, sorry to bother you again can you please review new changes .

@cjyuan
Copy link
Contributor

cjyuan commented Feb 17, 2025

Looks good. Feel free to mark this as completed.

@cjyuan cjyuan removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 17, 2025
@SabitaShrestha325
Copy link
Author

Looks good. Feel free to mark this as completed.

Thank you.

@SabitaShrestha325 SabitaShrestha325 added the Complete Volunteer to add when work is complete and all review comments have been addressed. label Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants